Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0] Add printing_auto_base #310

Open
wants to merge 8 commits into
base: 14.0
Choose a base branch
from

Conversation

jbaudoux
Copy link

@jbaudoux jbaudoux commented Dec 12, 2022

When a picking is done, automatically trigger the printing of some documents.
This can be used to print a delivery slip (report) or labels received from the carrier (attachment).

cc @sebalix @mt-software-de

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch 3 times, most recently from 0e4357b to 0202c43 Compare December 12, 2022 13:49
@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch 4 times, most recently from 4f0bba8 to 58bb18d Compare December 16, 2022 13:03
@jbaudoux
Copy link
Author

@mt-software-de I finished the refactor. Looks better now.

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch from 58bb18d to 9f047d5 Compare December 16, 2022 13:05
@sebalix sebalix added this to the 14.0 milestone Dec 16, 2022
@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch from 9f047d5 to 4ba3297 Compare December 16, 2022 13:11
Copy link

@mt-software-de mt-software-de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attachment field must be changed in the view and test to attachment_domain :-)

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch 3 times, most recently from 55daad4 to 9d3afab Compare December 16, 2022 13:50
@jbaudoux
Copy link
Author

jbaudoux commented Dec 16, 2022

Merged do_automated_printing and _do_automated_printing at https://github.com/OCA/report-print-send/pull/310/files#diff-ff122f5756c8074c5db95d9f9fcd541f871d3b6e312a3033b0e2304a50356676R57
Replaced Warning in favor of UserError

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch 10 times, most recently from 629569b to fdb2477 Compare December 16, 2022 15:20
@jbaudoux
Copy link
Author

Changes:

  • split into base & stock picking modules
  • added error logging
  • added button to redo the printing

@mt-software-de
Copy link

Changes:

* split into base & stock picking modules

* added error logging

* added button to redo the printing

Could you let me know when your are done with your changes?

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch from e0be80e to c14ee68 Compare December 16, 2022 15:31
@mt-software-de
Copy link

@jbaudoux what will happen with this PR, since OCA/stock-logistics-reporting#347 is merged?

@jbaudoux
Copy link
Author

jbaudoux commented Dec 6, 2024

@mt-software-de We should get this merged, this is more generic than the other. @sebalix Can you reopen it?

@sebalix sebalix reopened this Dec 6, 2024
@jbaudoux
Copy link
Author

jbaudoux commented Dec 6, 2024

Should printing_auto_stock_picking module move to stock-logistics-reporting and only keep the base here ? @sebalix @rousseldenis

Copy link

@mt-software-de mt-software-de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Beside the one test



@mock.patch.object(PrintingPrinter, "print_document", print_document)
@mock.patch.object(logging.Logger, "exception", exception)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should not be needed at all, since print_document is mocked.

@rousseldenis
Copy link
Contributor

Should printing_auto_stock_picking module move to stock-logistics-reporting and only keep the base here ? @sebalix @rousseldenis

@jbaudoux Yes, it makes sense.

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch 2 times, most recently from 42b597b to e645555 Compare December 6, 2024 11:56
@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch from e645555 to 221d2f8 Compare December 6, 2024 12:11
@jbaudoux jbaudoux changed the title [14.0] Add printing_auto_base + stock_picking_auto_print [14.0] Add printing_auto_base Dec 6, 2024
@jbaudoux
Copy link
Author

jbaudoux commented Dec 6, 2024

Should printing_auto_stock_picking module move to stock-logistics-reporting and only keep the base here ? @sebalix @rousseldenis

@jbaudoux Yes, it makes sense.

done

Copy link

@mt-software-de mt-software-de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@simahawk simahawk removed work in progress stale PR/Issue without recent activity, it'll be soon closed automatically. labels Dec 6, 2024
from odoo.exceptions import UserError, ValidationError

from .common import PrintingPrinter, TestPrintingAutoCommon, print_document
from .model_test import PrintingAutoTester, PrintingAutoTesterChild, setup_test_model
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbaudoux any reason not to use odoo_test_helper?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out that. I replaced it with your suggestion
cc @TDu

def _check_data_source(self):
for rec in self:
if rec.data_source == "report" and not rec.report_id:
raise UserError(_("Report is not set"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constrains should raise ValidaitonError

@jbaudoux jbaudoux force-pushed the 14-stock_picking_auto_print branch from c999790 to e5b0dee Compare December 11, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants